Kick ass! I'd love to see this feature.

We need explicit tests though to verify that an API version configured with N different strategies actually works correctly for all of the configured strategies simultaneously. Can you add a test for that? Should be as simple as adding a new multi strategy context here:

https://github.com/bploetz/versionist/blob/master/spec/api_routing_spec.rb#L72

Thanks.

Oh, and an example of this feature would be helpful in the README too.

Sure. I'll try to get that added tomorrow.

Thanks,
Josh

On Monday, July 23, 2012 at 5:34 PM, Brian Ploetz wrote:

Oh, and an example of this feature would be helpful in the README too.

---

Reply to this email directly or view it on GitHub:
#26 (comment)

Actually, this is going to be problematic if you only support a single :value key in the config hash, and you want your Header value to be different than your Request Parameter value. For example, you want to support both of these at the same time:

Accept: application/vnd.mycompany.com; version=1
GET /foos.json

GET /foos.json?version=1

Since the Header and Request Parameter strategies both take a :value config key, you won't be able to specify both at the same time.

  # header on it's own
  api_version(:module => "V1", :header => "Accept", :value => "application/vnd.mycompany.com; version=1") do
    match '/foos.(:format)' => 'foos#index', :via => :get
  end

  # request parameter on it's own
  api_version(:module => "V1", :parameter => "version", :value => "1") do
    match '/foos.(:format)' => 'foos#index', :via => :get
  end

  # both header and request parameter
  api_version(:module => "V1", :parameter => "version", :header => "Accept", :value => "?????") do
    match '/foos.(:format)' => 'foos#index', :via => :get
  end

I thought of this constraint, and for now I see it as a trade-off that a developer can chose. The Hypermedia purists probably won't chose it, but you can say that if you want to have multiple simultaneous strategies, then you'll probably want to use a custom header like your 'API-VERSION' suggestion that has a value of 'v1'.

At a minimum it's still fully backwards compatible, and gives people more choice, not less. Something more flexible (and possibly less backwards compatible) could be worked out when you're ready to do a major version bump?

  # both header and request parameter
  api_version(:module => "V1", :parameter => {:name => "version", :value => 'v1':}, :header => {:name => "Accept", :value => "application/vnd.mycompany.com; version=1"}) do
    match '/foos.(:format)' => 'foos#index', :via => :get
  end

Yeah, we should definitely fix the config ambiguity in a major version release, and I like the :strategy => {} approach to the config. Will get that on the road map.....

I added the specs and example. For the specs, i just put one positive test of each strategy. Putting more than that seemed to me like it would just be duplication of existing coverage, meaning slower specs without actually improving coverage.

This fails when you add :default => true to the config hash passed to api_version(), as it tries to set each strategy object created as the default, and you end up getting:

[VERSIONIST] attempt to set more than one default api version

Rather than shoe-horning this into the existing single config hash version of api_verison(), which clearly isn't set up to support this feature, I think I'd like to do this the right way by introducing the config hash per versioning strategy that you describe above, and doing this in a major version bump.

If you want to take a crack at that let me know, otherwise, I'll try to get going on that as soon as I can.

I completely understand your hesitation to "hacking in" something that was never intended to work instead of refactoring properly.

That said, I went ahead and added that case to the spec and made the one line change necessary for it to work. The shoe-horned method isn't really creating any code mess. I've only changed 5 lines total, and the changes were extremely minor.

I guess i'll just continue using my branch until one of us finds time for the refactoring :)

Has this made any advances? I'd love to see this functionality in the master

I have not forgotten about this. My wife and I welcomed a little girl into the world recently, so free time (and sleep) has been at a premium. I actually have the core refactoring done, I just need to finish updating the generators and it will be complete. I hope to push this up on a branch for you guys to play within a week or so.

Stay tuned....

I can't even begin to imagine, Brian! Congrats and good luck :)

This feature has been added in the release-1.0 branch. Before merging this to master and releasing version 1.0 with this change, I'd appreciate it if the folks interested in this feature could test this out with your API and let me know if you run into any issues. Change the dependency on versionist in your Gemfile to the following....

gem 'versionist', :git => 'git://github.com/bploetz/versionist.git', :branch => 'release-1.0'

...and run bundle update versionist.

Please read the "Uprgrading From Versionist 0.x to 1.x" section in the README for details:

https://github.com/bploetz/versionist/tree/release-1.0#upgrading-from-versionist-0x-to-1x

I tested with this:

api_version(module: 'V1', header: {name: 'API-VERSION', value: 'v1'}, parameter: {name: "version", value: 'v1'}, path: {value: 'v1'})

and it worked fine for me for all 3 methods.

Thanks, and congrats on the new child :)

Versionist 1.0.0 has been released with this change. Let me know if you run into any issues.